Overview

This PR addresses critical code quality issues identified during a comprehensive code review of the DOrcDeployModule based on industry best practices and PSScriptAnalyzer recommendations.

Critical Bug Fixed 🐛

Comparison Operator Bug in DeleteRabbit Function

Severity: HIGH - This bug would break the queue deletion functionality entirely.

Location: Line 698 in DeleteRabbit function

Issue: The code was using an assignment operator (=) instead of a comparison operator (-eq) in an elseif condition:

# Before (BUG):
elseif ($mode = 'queue') {
    # Queue deletion code
}

# After (FIXED):
elseif ($mode -eq 'queue') {
    # Queue deletion code
}

Impact: This bug would cause $mode to always be assigned the value 'queue', making the condition always evaluate to true regardless of the actual mode parameter value. This would break the exchange/queue mode logic and cause incorrect API calls to RabbitMQ.

Error Handling Improvements 🛡️

1. Empty Catch Blocks

Fixed three instances of empty catch blocks that were silently swallowing exceptions:

# Before:
catch { }

# After:
catch { 
    Write-Verbose "Could not retrieve OS information for $compName : $($_.Exception.Message)"
}

Functions affected: Invoke-RemoteProcess, Check-IsCitrixServer

2. Missing Error Handling

Added try-catch blocks and null checks to critical functions:

• SendEmailToDOrcSupport: Now catches SMTP failures and logs warnings
• CheckDiskSpace: Now handles WMI query failures and validates non-null results

# CheckDiskSpace example:
try {
    $ntfsVolumes = Get-WmiObject -Class win32_volume -cn $server | Where-Object {...}
}
catch {
    Write-Warning "Failed to query disk space on $serv : $($_.Exception.Message)"
    $bolSpaceCheckOK = $false
    continue
}
if (-not $ntfsVolumes) {
    Write-Warning "No NTFS volumes found on $serv"
    continue
}

3. Missing Else Branches

Added else clauses to handle edge cases:

• DeleteRabbit: Now warns when invalid mode values are provided
• CheckBackup: Now handles unexpected RestoreMode values after validation

Code Quality Improvements 📈

PowerShell Best Practices

Replaced cmdlet aliases with full cmdlet names for better code clarity and maintainability:

• where → Where-Object
• select → Select-Object
• icm → Invoke-Command

Locations: 10+ instances throughout the module

Test Coverage ✅

Added comprehensive unit tests using Pester to validate the critical bug fixes:

New Tests

1. DeleteRabbit Mode Parameter Tests (3 tests)

   • Validates 'exchange' mode works correctly
   • Validates 'queue' mode works correctly
   • Validates invalid mode handling

2. CheckBackup RestoreMode Tests (1 test)

   • Validates invalid RestoreMode throws appropriate error

Test Results: 4/4 passing (100% pass rate)

Metrics

Impact

These changes significantly improve:

• Reliability: Critical bug fix prevents incorrect RabbitMQ operations
• Debuggability: Proper error logging helps troubleshoot issues in production
• Maintainability: Following PowerShell best practices makes code easier to understand
• Quality Assurance: Test coverage ensures bugs stay fixed

Files Changed

• DOrcDeployModule.psm1 - Core module with critical fixes (67 lines modified)
• DOrcDeployModule.tests.ps1 - Enhanced test coverage (39 lines added)

Breaking Changes

None. All changes are backward compatible.

Testing Instructions

1. Run existing tests: Invoke-Pester -Path .\DOrcDeployModule.tests.ps1
2. Run new tests specifically: Invoke-Pester -Path .\DOrcDeployModule.tests.ps1 -FullName '*DeleteRabbit*', '*CheckBackup*'
3. Validate syntax: Test-Path .\DOrcDeployModule.psm1 and check for no parsing errors

Review Notes

This PR resolves all critical issues identified during the code review. Remaining warnings (476) are mostly related to unused variables and function naming conventions, which are low priority and don't affect functionality.

Fixes #6

Fixes #6

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.